Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changed: Prevent possible FileStore Deadlocks on GC By Preventing Yielding to external code. #2063

Merged
merged 6 commits into from
Sep 19, 2024

Conversation

Sewer56
Copy link
Member

@Sewer56 Sewer56 commented Sep 19, 2024

Very quick patch.

Updating the DataBase's file copy of the FileStore objects interacts with external non-GC components,
such as MnemonicDB. This may cause us to yield to external code that could touch the FileStore lock.

To avoid deadlocks, we should prevent this from happening if possible. This is why we release store.Lock() early
and instead put another lock on the GC operation itself.


Long term, we may want to update IFileStore a bit to be resilient when opening streams. For additional comments see:
#2058 (comment)

In addition, we still need a non-blocking Commit in MMDB; I put the details surrounding that in the code itself.

@Sewer56 Sewer56 added Bug Something isn't working area-filestore labels Sep 19, 2024
@Sewer56 Sewer56 requested a review from a team September 19, 2024 14:24
@Sewer56 Sewer56 self-assigned this Sep 19, 2024
@Sewer56 Sewer56 marked this pull request as draft September 19, 2024 14:32
@Sewer56 Sewer56 marked this pull request as ready for review September 19, 2024 14:52
@Al12rs
Copy link
Contributor

Al12rs commented Sep 19, 2024

Seems to be working correctly with these changes

@Al12rs Al12rs merged commit 5fe29ca into main Sep 19, 2024
15 of 16 checks passed
@Al12rs Al12rs deleted the temp-un-deadlock-filestore branch September 19, 2024 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-filestore Bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants